Adjusted key_setup() to fix my issue #110#111
Adjusted key_setup() to fix my issue #110#111drithird wants to merge 2 commits intocodemation:mainfrom
Conversation
…is inline with the quick start setup
Fixed an oversite where I was preappending key to the directory path
|
|
||
| if key_path: | ||
| key_path = f"{key_path}/{os.environ['KEY_NAME']}.key" | ||
| key_path = f"{key_path}/{os.environ['KEY_NAME']}" |
There was a problem hiding this comment.
Removing the .key postfix is probably a good idea in the long run, but is a breaking change for anyone defining the KEY_NAME without .key.
| assert "KEY_NAME" in os.environ, f"missing KEY_NAME env variable" | ||
| key_path = os.getenv("KEY_PATH", "") | ||
|
|
||
| if not os.path.exists(key_path): |
There was a problem hiding this comment.
I like the idea of adding a check here for the path, but raising an exception instead of creating the path.
The reason behind this is that with the ephemeral workers like docker / kubernetes, we would have a different private key each a new instance started instead of forcing the developer to ensure that a persistent volume is used to store the private key. For additional context, when a private key changes any existing and unexpired tokens will not work with a newly refreshed private key.
| key_path = f"{os.environ['KEY_NAME']}" | ||
|
|
||
| if not os.path.exists(key_path): | ||
| with open(key_path, "w") as file: |
There was a problem hiding this comment.
Once the directory exists, there is is no need to make an empty file.
2 participants
I adjusted the key_setup function so that it now checks to see if both the directory and file are created before proceeding with the rest of the code in the event that no key exists.
Let me know if you have any questions.